Sorry reviewers, I would love to add a test for this change, but I cannot think of a good way to do it. If you've got some ideas, I'd love to implement them.

CircleCI failed for some obscure reason that I cannot understand, I guess its not related to the change I've made?

Hey @xiaket - thanks for this PR! A couple of things:

1. The reason you are getting the error in tests is because your stacker fork master branch is very old - it's from back in october of 2017:

If you update it from the current master in remind101/stacker, and then push your changes that should clear this up.

2. There are a couple of ways to write tests for this - first, it might be good to create a generic function for getting the region for lookups that matches what this does, then you can test it with various permutations. Second, if you want to test actual queries against the AWS API, you can do so with the botocore.Stubber class. You can find docs here: http://botocore.readthedocs.io/en/latest/reference/stubber.html and plenty of examples of it's use in https://github.com/remind101/stacker/tree/f8f83c3ab50cd454e8aff2990b94b28b52fd5f04/stacker/tests

Let me know if you have any issues digging into either of these, and thanks again for the PR!

Thanks @phobologic ! I had tried to make a little contribution back then and I totally forget about it until you pointed out, that's a stupid mistake.

Anyway, I've also added the tests, hope that will make sense.

Hey @xiaket - sorry for not getting back to you sooner. This PR actually made me realize that we potentially have a bit bigger of an issue - mostly that our lookups don't use any common way of determining what region to use. I think that might be a good thing to do going forward - and would be more easily testable in general (really, the logic isn't around if we use the right end point provided with a region - the logic is how our lookups CHOOSE their region).

I'm wondering if we should instead have a common helper method for determining the region used by lookups? Going to ping some other folks to get their opinion here - @troyready @ejholmes @danielkza @GarisonLotus @russellballestrini @Lowercases

Thanks, @phobologic for your reply! I agree, adding a new general way to choose the aws region would be really useful.

I'd prefer this to go into a 2.0 branch, since it technically changes existing behavior (there may be users deploying to oregon/etc that have parameters in virginia and are relying on the default lookup).

Agree on the idea of adding a common method as well. This implementation seems fine for now - not sure how it interacts with the new multi-profile/region support?

Codecov Report

❗ No coverage uploaded for pull request base (master@a741a90). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff            @@
##             master    #595   +/-   ##
========================================
  Coverage          ?   89.4%           
========================================
  Files             ?      92           
  Lines             ?    5730           
  Branches          ?       0           
========================================
  Hits              ?    5123           
  Misses            ?     607           
  Partials          ?       0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a741a90...3a3aaab. Read the comment docs.